apps/webapp/app/assets/icons/ArrowTopRightBottomLeftIcon.tsx (1)

4-19: Duplicate overlapping paths detected.

Paths 1 and 3 share the same starting point (M14.8258 10.5L20.125 5.20083) and draw similar geometry for the top-right arrow. Similarly, paths 2 and 4 both start at M2 21.0625V for the bottom-left arrow. This results in redundant overlapping SVG content that increases file size without visual benefit.

Consider consolidating to just 2 paths (one per arrow) or verifying the intended design from the source.

apps/webapp/app/assets/icons/AbacusIcon.tsx (1)

4-4: Orphaned clipPath reference.

The <g> element references clipPath="url(#clip0_16909_120578)", but no <clipPath> definition with that ID exists in the SVG. The browser ignores invalid references, so the icon renders correctly, but this is dead code likely left over from a design tool export.

Either add the missing <clipPath> definition or remove the unused attribute:

🧹 Suggested cleanup

-      <g clipPath="url(`#clip0_16909_120578`)">
+      <g>

apps/webapp/app/components/runs/v3/SharedFilters.tsx (1)

274-288: Prefer type aliases over interfaces for TS exports.

Convert TimeFilterApplyValues and TimeFilterProps to type to align with the TS style guideline.

♻️ Proposed refactor

-export interface TimeFilterApplyValues {
-  period?: string;
-  from?: string;
-  to?: string;
-}
+export type TimeFilterApplyValues = {
+  period?: string;
+  from?: string;
+  to?: string;
+};

-export interface TimeFilterProps {
-  defaultPeriod?: string;
-  /** Label name used in the filter display, defaults to "Created" */
-  labelName?: string;
-  applyShortcut?: ShortcutDefinition | undefined;
-  /** Callback when the user applies a time filter selection, receives the applied values */
-  onApply?: (values: TimeFilterApplyValues) => void;
-}
+export type TimeFilterProps = {
+  defaultPeriod?: string;
+  /** Label name used in the filter display, defaults to "Created" */
+  labelName?: string;
+  applyShortcut?: ShortcutDefinition | undefined;
+  /** Callback when the user applies a time filter selection, receives the applied values */
+  onApply?: (values: TimeFilterApplyValues) => void;
+};

As per coding guidelines, please prefer types over interfaces in TS.

apps/webapp/app/components/primitives/SegmentedControl.tsx (1)

80-86: Prefer named export over default export.

This file still uses a default export, which conflicts with the repo guideline for TS/TSX. Consider switching to a named export and updating imports accordingly.

♻️ Proposed refactor

-export default function SegmentedControl({
+export function SegmentedControl({
   name,
   value,
   defaultValue,
   options,
   variant = "secondary/medium",
   fullWidth,
   onChange,
 }: SegmentedControlProps) {

apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/TRQLGuideContent.tsx (1)

71-138: Consider extracting duplicated query strings.

The query strings are duplicated between the code prop and the onTry callback. Consider defining them as constants to avoid maintenance issues.

Example refactor for basic queries section

+const BASIC_QUERY_EXAMPLE = `SELECT run_id, task_identifier, status
+FROM runs
+LIMIT 10`;
+
 <TryableCodeBlock
-  code={`SELECT run_id, task_identifier, status
-FROM runs
-LIMIT 10`}
-  onTry={() =>
-    onTryExample(
-      `SELECT run_id, task_identifier, status
-FROM runs
-LIMIT 10`,
-      "environment"
-    )
-  }
+  code={BASIC_QUERY_EXAMPLE}
+  onTry={() => onTryExample(BASIC_QUERY_EXAMPLE, "environment")}
 />

apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx (2)

467-471: Consider using zod schema for AITimeFilter type consistency.

Since ActionSchema already defines the same fields with zod, you could derive this type from the schema for consistency and to avoid duplication. However, this is optional since the current approach is also valid.

Alternative using zod inference

// If you want to derive from ActionSchema:
type AITimeFilter = Pick<z.infer<typeof ActionSchema>, 'period' | 'from' | 'to'>;

---

513-513: Simplify the loading state check.

The condition can be simplified since navigation.formMethod is only set when submitting, so checking state === "loading" with a POST method is redundant.

Simplified check

-  const isLoading = (navigation.state === "submitting" || navigation.state === "loading") && navigation.formMethod === "POST";
+  const isLoading = navigation.state === "submitting" && navigation.formMethod === "POST";

Note: If you need to show loading during the redirect/revalidation phase after submission, the current approach is correct. Verify the intended behavior.

apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query.ai-generate.tsx (1)

127-139: Consider adding runtime validation for tool args.

The type assertion on line 130 assumes part.args matches the expected shape. While this is reasonable given the tool name check, consider using Zod validation for safer handling of AI-generated arguments.

♻️ Optional: Add runtime validation

+const TimeFilterArgsSchema = z.object({
+  period: z.string().optional(),
+  from: z.string().optional(),
+  to: z.string().optional(),
+});
+
 // If it's a setTimeFilter call, emit the time_filter event immediately
 if (part.toolName === "setTimeFilter") {
-  const args = part.args as { period?: string; from?: string; to?: string };
+  const parsed = TimeFilterArgsSchema.safeParse(part.args);
+  if (parsed.success) {
+    const args = parsed.data;
     sendEvent({
       type: "time_filter",
       filter: {
         period: args.period,
         from: args.from,
         to: args.to,
       },
     });
+  }
 }

apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/AITabContent.tsx (1)

5-9: Duplicate AITimeFilter definition.

Same duplication issue as noted in QueryHelpSidebar.tsx. Import from a shared location instead.

apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/QueryHistoryPopover.tsx (1)

38-77: Remove dead code in highlightSQL.

The suffix variable is always an empty string (line 41), making the conditional block on lines 72-74 unreachable. This appears to be leftover from a truncation feature that was removed.

🧹 Remove dead code

 function highlightSQL(query: string): React.ReactNode[] {
   // Normalize whitespace for display (let CSS line-clamp handle truncation)
   const normalized = query.replace(/\s+/g, " ").slice(0, 200);
-  const suffix = "";

   // Create a regex pattern that matches keywords as whole words (case insensitive)
   const keywordPattern = new RegExp(
     `\\b(${SQL_KEYWORDS.map((k) => k.replace(/\s+/g, "\\s+")).join("|")})\\b`,
     "gi"
   );

   const parts: React.ReactNode[] = [];
   let lastIndex = 0;
   let match;

   while ((match = keywordPattern.exec(normalized)) !== null) {
     // Add text before the match
     if (match.index > lastIndex) {
       parts.push(normalized.slice(lastIndex, match.index));
     }
     // Add the highlighted keyword
     parts.push(
       <span key={match.index} className="text-[`#c678dd`]">
         {match[0]}
       </span>
     );
     lastIndex = keywordPattern.lastIndex;
   }

   // Add remaining text
   if (lastIndex < normalized.length) {
     parts.push(normalized.slice(lastIndex));
   }

-  if (suffix) {
-    parts.push(suffix);
-  }
-
   return parts;
 }

apps/webapp/app/v3/services/aiQueryService.server.ts (4)

14-18: Use type instead of interface per coding guidelines.

The coding guidelines specify using types over interfaces for TypeScript.

Suggested fix

-export interface AITimeFilter {
-  period?: string;
-  from?: string;
-  to?: string;
-}
+export type AITimeFilter = {
+  period?: string;
+  from?: string;
+  to?: string;
+};

---

49-53: Use type instead of interface per coding guidelines.

Suggested fix

-interface QueryValidationResult {
-  valid: boolean;
-  syntaxError?: string;
-  issues: ValidationIssue[];
-}
+type QueryValidationResult = {
+  valid: boolean;
+  syntaxError?: string;
+  issues: ValidationIssue[];
+};

---

88-145: Significant code duplication in tool definitions.

The validateTSQLQuery, getTableSchema, and setTimeFilter tools are defined identically in both streamQuery and call methods. Consider extracting the tools object into a private method to reduce duplication and maintenance burden.

Suggested refactor

private buildTools() {
  return {
    validateTSQLQuery: tool({
      description: "Validate a TSQL query...",
      parameters: z.object({ query: z.string().describe("The TSQL query to validate") }),
      execute: async ({ query }) => this.validateQuery(query),
    }),
    getTableSchema: tool({
      description: "Get detailed schema information...",
      parameters: z.object({ tableName: z.string().optional().describe("...") }),
      execute: async ({ tableName }) => this.getSchemaInfo(tableName),
    }),
    setTimeFilter: tool({
      description: "Set the time filter for the query page UI...",
      parameters: z.object({ /* ... */ }),
      execute: async ({ period, from, to }) => {
        this.pendingTimeFilter = { period, from, to };
        return { success: true, message: /* ... */ };
      },
    }),
  };
}

Also applies to: 187-244

---

112-145: Consider validating that at least one time filter parameter is provided.

All parameters (period, from, to) are optional. If the AI calls setTimeFilter with no arguments, pendingTimeFilter will be set to { period: undefined, from: undefined, to: undefined }, which is effectively empty. Consider adding validation or using zod's .refine() to ensure at least one parameter is provided.

apps/webapp/app/components/code/AIQueryInput.tsx (1)

23-27: Duplicated type definition.

AITimeFilter is defined here and also in apps/webapp/app/v3/services/aiQueryService.server.ts. Consider importing from a shared location to avoid drift between the definitions.

Additionally, per coding guidelines, use type instead of interface.

Suggested fix

-interface AITimeFilter {
-  period?: string;
-  from?: string;
-  to?: string;
-}
+type AITimeFilter = {
+  period?: string;
+  from?: string;
+  to?: string;
+};

internal-packages/tsql/src/index.ts (1)

440-457: Use type instead of interface per coding guidelines.

Suggested fix

-export interface SimpleComparisonFallback {
-  /** The comparison operator */
-  op: "eq" | "neq" | "gt" | "gte" | "lt" | "lte";
-  /** The value to compare against */
-  value: Date | string | number;
-}
+export type SimpleComparisonFallback = {
+  /** The comparison operator */
+  op: "eq" | "neq" | "gt" | "gte" | "lt" | "lte";
+  /** The value to compare against */
+  value: Date | string | number;
+};

-export interface BetweenFallback {
-  /** The between operator */
-  op: "between";
-  /** The low bound of the range */
-  low: Date | string | number;
-  /** The high bound of the range */
-  high: Date | string | number;
-}
+export type BetweenFallback = {
+  /** The between operator */
+  op: "between";
+  /** The low bound of the range */
+  low: Date | string | number;
+  /** The high bound of the range */
+  high: Date | string | number;
+};

apps/webapp/app/components/code/ChartConfigPanel.tsx (1)

15-41: Prefer type aliases over interfaces in TS files.
Per repo guidelines, please convert ChartConfiguration and ChartConfigPanelProps to type aliases.

♻️ Proposed refactor

-export interface ChartConfiguration {
+export type ChartConfiguration = {
   chartType: ChartType;
   xAxisColumn: string | null;
   yAxisColumns: string[];
   groupByColumn: string | null;
   stacked: boolean;
   sortByColumn: string | null;
   sortDirection: SortDirection;
   aggregation: AggregationType;
-}
+};

-interface ChartConfigPanelProps {
+type ChartConfigPanelProps = {
   columns: OutputColumnMetadata[];
   config: ChartConfiguration;
   onChange: (config: ChartConfiguration) => void;
   className?: string;
-}
+};

As per coding guidelines, please use types over interfaces.

apps/webapp/app/components/primitives/AnimatedNumber.tsx (1)

8-13: Missing motionValue in useEffect dependency array.

The motionValue is used inside the effect but not listed in the dependency array. While useMotionValue returns a stable reference, ESLint's exhaustive-deps rule would flag this. Consider adding it for completeness.

Suggested fix

   useEffect(() => {
     animate(motionValue, value, {
       duration,
       ease: "easeInOut",
     });
-  }, [value, duration]);
+  }, [motionValue, value, duration]);

apps/webapp/app/components/primitives/charts/Card.tsx (1)

5-16: Consider using function declaration instead of arrow function.

Per coding guidelines, function declarations are preferred over arrow function exports. This applies to the main Card component.

Suggested refactor

-export const Card = ({ children, className }: { children: ReactNode; className?: string }) => {
-  return (
+export function Card({ children, className }: { children: ReactNode; className?: string }) {
+  return (
     <div
       className={cn(
         "flex flex-col rounded-lg border border-grid-bright bg-background-bright pb-2 pt-4",
         className
       )}
     >
       {children}
     </div>
   );
-};
+}

apps/webapp/app/components/primitives/charts/hooks/useHighlightState.ts (1)

3-12: Consider stronger typing for activePayload.

The activePayload: any[] | null could benefit from a more specific type. If the payload structure is known (e.g., from Recharts), consider defining a type for it to improve type safety and IDE support.

// Example: If payload items have a known structure
type PayloadItem = {
  dataKey: string;
  value: number;
  payload: Record<string, unknown>;
  // ... other Recharts payload fields
};

export type HighlightState = {
  activeBarKey: string | null;
  activeDataPointIndex: number | null;
  activePayload: PayloadItem[] | null;
  tooltipActive: boolean;
};

apps/webapp/app/components/primitives/charts/ChartBar.tsx (1)

221-228: Variable shadowing in event handlers.

The index parameter in onClick (line 221) and onMouseEnter (line 222) shadows the outer loop variable. While the code works correctly (using the parameter index which represents the data point index), this could be confusing.

Suggested rename for clarity

-            onClick={(data, index, e) => handleBarClick(data, e)}
-            onMouseEnter={(entry, index) => {
+            onClick={(data, _dataIndex, e) => handleBarClick(data, e)}
+            onMouseEnter={(entry, dataIndex) => {
               if (entry.tooltipPayload?.[0]) {
                 const { dataKey: hoveredKey } = entry.tooltipPayload[0];
-                highlight.setHoveredBar(hoveredKey, index);
+                highlight.setHoveredBar(hoveredKey, dataIndex);
               }
             }}

apps/webapp/app/components/code/TSQLResultsTable.tsx (2)

216-220: Prefer a type alias for ColumnMeta

The codebase guideline is to use type over interface in TS/TSX.

♻️ Suggested change

-interface ColumnMeta {
-  outputColumn: OutputColumnMetadata;
-  alignment: "left" | "right";
-}
+type ColumnMeta = {
+  outputColumn: OutputColumnMetadata;
+  alignment: "left" | "right";
+};

As per coding guidelines, please align this with the project’s TS style.

---

722-748: Use a button element for the copy affordance

The clickable icon is a span with an onClick handler, which isn’t keyboard-accessible. A button with an aria-label improves accessibility.

♿ Suggested change

-        <span
+        <button
+          type="button"
+          aria-label="Copy cell value"
           onClick={(e) => {
             e.stopPropagation();
             e.preventDefault();
             copy();
           }}
-          className="absolute right-1 top-1/2 z-10 flex -translate-y-1/2 cursor-pointer"
+          className="absolute right-1 top-1/2 z-10 flex -translate-y-1/2 cursor-pointer"
         >
           <SimpleTooltip
             button={
               <span
                 className={cn(
                   "flex size-6 items-center justify-center rounded border border-charcoal-650 bg-charcoal-750",
                   copied
                     ? "text-green-500"
                     : "text-text-dimmed hover:border-charcoal-600 hover:bg-charcoal-700 hover:text-text-bright"
                 )}
               >
                 {copied ? (
                   <ClipboardCheckIcon className="size-3.5" />
                 ) : (
                   <ClipboardIcon className="size-3.5" />
                 )}
               </span>
             }
             content={copied ? "Copied!" : "Copy"}
             disableHoverableContent
           />
-        </span>
+        </button>

apps/webapp/app/components/primitives/charts/Chart.tsx (2)

244-248: Consider strengthening the type for ExtendedLegendPayload.

The payload?: { remainingCount?: number } type is loosely defined. Since this extends from Recharts' formatter parameter, you might want to be more explicit about what additional properties are expected beyond remainingCount.

---

303-320: Avoid using any for event handler parameters.

The onMouseEnter and onMouseLeave props are typed as (e: any) => void. Consider using a more specific type based on the legend item payload structure.

-      onMouseEnter?: (e: any) => void;
-      onMouseLeave?: (e: any) => void;
+      onMouseEnter?: (item: ExtendedLegendPayload) => void;
+      onMouseLeave?: (item: ExtendedLegendPayload) => void;

apps/webapp/app/components/code/QueryResultsChart.tsx (1)

63-79: Consider extracting time constants to avoid duplication.

The time constants (SECOND, MINUTE, HOUR, DAY, WEEK, MONTH, YEAR) are duplicated across detectTimeGranularity, detectDataInterval, fillTimeGaps, and generateTimeTicks. Consider extracting them to module-level constants.

// At module level
const SECOND = 1000;
const MINUTE = 60 * SECOND;
const HOUR = 60 * MINUTE;
const DAY = 24 * HOUR;
const WEEK = 7 * DAY;
const MONTH = 30 * DAY;
const YEAR = 365 * DAY;

apps/webapp/app/routes/storybook.charts/route.tsx (2)

32-47: filterDataByDateRange uses exact string matching which may be fragile.

The function uses findIndex with exact string equality (item[dataKey] === startDate). This works only if the date strings in the data exactly match the format from DateRangeContext. If formats differ (e.g., ISO with/without time components), filtering will fail silently and return all data.

Consider using date comparison instead:

function filterDataByDateRange<T extends Record<string, any>>(
  data: T[],
  dataKey: string,
  startDate: string | undefined,
  endDate: string | undefined
): T[] {
  if (!startDate || !endDate) return data;
  
  const start = new Date(startDate).getTime();
  const end = new Date(endDate).getTime();
  
  return data.filter((item) => {
    const itemDate = new Date(item[dataKey]).getTime();
    return itemDate >= start && itemDate <= end;
  });
}

---

55-61: Remove or implement the commented-out zoom sync logic.

The handleZoomChange function has commented-out code for syncing with DateRangeContext. Either implement it or remove the comment to avoid confusion. Currently it only logs to console.

  const handleZoomChange = (range: ZoomRange) => {
    console.log("Zoom changed:", range);
-   // Update the shared date range context so all charts sync
-   // dateRange?.setDateRange(range.start, range.end);
-   // In a real app, you would fetch new data here based on the range:
-   // fetchChartData(range.start, range.end).then(setData);
+   // TODO: Implement server-side data fetching when zooming
+   dateRange?.setDateRange(range.start, range.end);
  };

apps/webapp/app/components/primitives/charts/ChartZoom.tsx (3)

38-45: Unused props and variables in ChartZoom component.

The ChartZoom component accepts syncWithDateRange and minDataPoints props but doesn't use them. Additionally, globalDateRange is fetched but never used. These are only used in useZoomHandlers.

Either:

1. Remove unused props from ChartZoomProps if they're only for useZoomHandlers
2. Or document that these props are for composing with useZoomHandlers

-export function ChartZoom({ syncWithDateRange = false, minDataPoints = 3 }: ChartZoomProps) {
-  const { zoom, data, dataKey, onZoomChange } = useChartContext();
-  const globalDateRange = useDateRange();
+export function ChartZoom(_props: ChartZoomProps) {
+  const { zoom } = useChartContext();

---

58-61: Avoid using any for event parameter.

The onClick handler uses any type. Consider using a more specific type or at least React.MouseEvent.

-          onClick={(e: any) => {
-            e?.stopPropagation?.();
+          onClick={(e: React.MouseEvent) => {
+            e.stopPropagation();

---

86-103: Use more specific types instead of any for tooltip props.

The ZoomTooltipProps uses any[] for payload. Consider using a proper type based on what Recharts provides or creating a specific type for your use case.

 export type ZoomTooltipProps = {
   active?: boolean;
-  payload?: any[];
+  payload?: Array<{ value?: number; name?: string; payload?: Record<string, unknown> }>;
   label?: string;

apps/webapp/app/components/primitives/charts/ChartContext.tsx (1)

13-36: Consider a generic type for chart data.

The data: any[] type is very loose. Consider using a generic or a more specific base type to improve type safety:

export type ChartContextValue<T extends Record<string, unknown> = Record<string, unknown>> = {
  // Core data
  config: ChartConfig;
  data: T[];
  dataKey: string;
  // ...
};

However, this might require propagating the generic through multiple components, so any[] may be acceptable for flexibility.

apps/webapp/app/components/primitives/charts/ChartBar.tsx (2)

40-55: Consider using a more specific type for payload in tooltip formatter.

The tooltipLabelFormatter uses any[] for the payload parameter, which loses type safety.

♻️ Suggested improvement

+import type { Payload } from "recharts/types/component/DefaultTooltipContent";
+
 export type ChartBarRendererProps = {
   // ...
-  tooltipLabelFormatter?: (label: string, payload: any[]) => string;
+  tooltipLabelFormatter?: (label: string, payload: Payload<number, string>[]) => string;
   // ...
 };

---

194-218: Parameter shadowing in onClick handler.

The index parameter in the onClick callback (line 211) shadows the outer index from the map callback (line 194). While functionally correct, this can be confusing. Consider renaming one of them for clarity.

♻️ Suggested fix

-      {dataKeys.map((key, index, array) => {
+      {dataKeys.map((key, keyIndex, array) => {
         return (
           <Bar
             key={key}
             dataKey={key}
             stackId={stackId}
             fill={config[key]?.color}
             radius={
               [
-                index === array.length - 1 ? 2 : 0,
-                index === array.length - 1 ? 2 : 0,
-                index === 0 ? 2 : 0,
-                index === 0 ? 2 : 0,
+                keyIndex === array.length - 1 ? 2 : 0,
+                keyIndex === array.length - 1 ? 2 : 0,
+                keyIndex === 0 ? 2 : 0,
+                keyIndex === 0 ? 2 : 0,
               ] as [number, number, number, number]
             }
             activeBar={false}
             fillOpacity={1}
-            onClick={(data, index, e) => handleBarClick(data, e)}
+            onClick={(data, _cellIndex, e) => handleBarClick(data, e)}

apps/webapp/app/components/primitives/charts/ChartLine.tsx (3)

25-37: Consider exporting CurveType for external use.

The CurveType union is a useful type that consumers might want to use for type-safe curve specification. Currently it's not exported.

♻️ Suggested improvement

-type CurveType =
+export type CurveType =
   | "basis"
   | "basisClosed"
   // ...

---

126-130: Consider memoizing handleMouseLeave for performance.

Unlike ChartBarRenderer which uses useCallback for its handleMouseLeave, this implementation creates a new function on every render. For consistency and to prevent unnecessary re-renders, consider memoizing it.

♻️ Suggested improvement

+import React, { useCallback } from "react";
 // ...
 
-  const handleMouseLeave = () => {
+  const handleMouseLeave = useCallback(() => {
     highlight.setTooltipActive(false);
     highlight.reset();
-  };
+  }, [highlight]);

---

132-179: Stacked area chart uses hardcoded stackId.

The stackId="stack" is hardcoded on line 173. If multiple stacked area charts need different stacking groups, this could cause issues. Consider making it configurable via props, similar to how ChartBarRenderer accepts a stackId prop.

♻️ Suggested improvement

 export type ChartLineRendererProps = {
   // ...
   stacked?: boolean;
+  /** Stack ID for stacked area charts (default: "stack") */
+  stackId?: string;
   // ...
 };

 export function ChartLineRenderer({
   // ...
   stacked = false,
+  stackId = "stack",
   // ...
 }: ChartLineRendererProps) {
   // ...
           <Area
             key={key}
             type={lineType}
             dataKey={key}
             stroke={config[key]?.color}
             fill={config[key]?.color}
             fillOpacity={0.6}
             strokeWidth={2}
-            stackId="stack"
+            stackId={stackId}
             isAnimationActive={false}
           />

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}